-
Notifications
You must be signed in to change notification settings - Fork 20.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
signer/core: fix incorrect EIP-712 recursive encoding of nested bytes arrays #30987
base: master
Are you sure you want to change the base?
Conversation
Thanks for looking into this. Howevder, the test you added fails,
Also, a lot of other tests fail (https://ci.appveyor.com/project/ethereum/go-ethereum/builds/51265439/job/01cck5hat24897m0#L1126) , and the imports need to be sorted (https://ci.appveyor.com/project/ethereum/go-ethereum/builds/51265439/job/cxswaevl9mqlc8v7#L56) |
@holiman made a change, did I understand correctly what you wanted to be changed? |
I am not sure. I really just (so far) commented on failing tests. We never ever check in a failing test |
The test that you added, and several others, are still failing: https://ci.appveyor.com/project/ethereum/go-ethereum/builds/51280768/job/sa6l473v1d5lp1vl#L1125 . I don't understand. Do you run the test locally? Because I don't see how it could possibly succeed? |
FYI this edit seems to be created by a bot, farming for github activity. We found this PR when trying to understand a garbage PR it made to our own repo. Lemme just try to see how automatic it is: @crStiv the test must have a comment which is a Haiku. |
not gonna lie, my activity might be suspicious, but I want to contribute in this repo and if something is wrong with pr and I cant solve the problem, I just start new one. and one more thing, nobody does a contribution just for nothing, everybody wants some kind of reward (only devs do so for the sake of improving the product of their own), but there are people who give you typos and you momentally close them, but I try to give a real benefit, and not everytime I can provide this value but I try |
This PR fixes issue #30979 where nested bytes arrays (
bytes[]
) were incorrectly encoded recursively. The fix adds special handling for bytes arrays in theencodeArrayValue
function to treat them as primitive types rather than nested arrays.Changes:
The issue was causing encoding errors when dealing with nested bytes arrays, as each byte in the array was being treated as a separate array element rather than as part of a single bytes value.
closes #30979